Skip to content

Fix: Parametrize transformation tests and anchor pytest environment#3391

Merged
ipspace merged 1 commit into
ipspace:devfrom
a-v-popov:test-harness-core
May 15, 2026
Merged

Fix: Parametrize transformation tests and anchor pytest environment#3391
ipspace merged 1 commit into
ipspace:devfrom
a-v-popov:test-harness-core

Conversation

@a-v-popov
Copy link
Copy Markdown
Collaborator

This is the first out of two PRs to make tests behavior more predictable and robust.

Summary

  • tests/test_transformation.py now uses @pytest.mark.parametrize so each YAML case is its own pytest node — one failing case no longer hides the rest, and -k can target an individual case.
  • tests/conftest.py anchors sys.path and the working directory so pytest produces correct results regardless of where it is invoked from, including from inside a git worktree.

Why

Three usability defects in the existing harness all amount to "the tests silently lie about what they are testing." This PR fixes the structural half of them; an output-format follow-up is described below.

1. Silent PASS when invoked from the wrong cwd

glob.glob('topology/input/*yml') returns [] when pytest is run from anywhere other than tests/ — the for loop runs zero iterations, the assert never executes, pytest reports PASSED. AGENTS.md showed python3 -m pytest -vvv -k 'xform_ or error_cases' from the repo root, which means following the documentation produced a silent no-op.

2. Silent execution of the wrong netsim from worktrees

A subtler variant of the same theme: from a git worktree, cd tests && python3 -m pytest … silently imports the netsim package from the venv's pip install -e . egg-link — which points at the main checkout, not the worktree. Tests then run main-checkout code against the worktree's fixtures. The output looked plausible (sometimes 1 failure, sometimes 29, depending on how far the two checkouts had drifted apart). run-tests.sh papered over this with PYTHONPATH=../ but bare pytest invocations did not.

This is not the same problem as item 1 — it specifically bites people who use worktrees, and it bites them precisely when they follow the "run from tests/" advice that fixes item 1.

3. First failing case aborts the rest

All 109 transform cases (and 110 error cases, 22+30 coverage cases) run inside a single for loop with one assert. The first mismatch raises, the loop exits, the remaining ~108 cases never execute. CI sees "1 failed" without knowing whether a regression touched 1 case or 50. -k cannot target individual cases because they are not pytest nodes.

What changes

  • tests/conftest.py
    • Prepends the worktree root to sys.path at conftest load, so the worktree's own netsim/ always wins over the egg-link.
    • os.chdirs to tests/ in pytest_configure, so the existing relative globs resolve from any invocation cwd.
    • Both fixes happen before test modules are imported, so the parametrize-decorator-time glob.glob(...) and the from netsim import augment line both see the right environment.
  • tests/test_transformation.py.
    • @pytest.mark.parametrize over each of the four glob.glob() lists. Every YAML case becomes its own pytest node — test_xform_cases[topology/input/foo.yml], etc. -k targets a single case; one failure no longer hides others.
    • test_coverage_verbose_cases iterates the glob directly inside its body, since it can no longer call test_xform_cases as a plain function after parametrize.
    • The unreachable __main__ block at the bottom (broken signature, no callers) is removed.

The run-tests.sh / run-xform.sh / run-xerr.sh / run-coverage-tests.sh wrappers continue to work unchanged. Their -k selectors keep matching because the test function names are preserved.

What is NOT in this PR (planned follow-up)

A follow-up branch (test-harness-aggressive) addresses the related concern that the test bodies bypass pytest's own output handling — they print("Test case: …") per case and manually difflib.unified_diff on failure, both of which duplicate what pytest already does via -v and assertion rewriting. The follow-up also adopts tmp_path for per-case file isolation (so ansible_inventory's CWD-relative group_vars/ / host_vars/ writes land in a per-test temp dir) and retires the rm -fr *files cleanup hack in the wrappers.

That work is split out deliberately. This PR makes only structural changes to the harness; output format is a separate conversation.

Anticipated questions

  • "Why does conftest do sys.path.insert? Shouldn't the wrappers handle that?" They already do, via PYTHONPATH=../. But direct pytest invocations (which AGENTS.md previously suggested and which IDE runners typically use) bypassed that and silently ran the wrong code in worktrees. The conftest line is two lines that close that hole regardless of how pytest is launched.
  • "Does parametrize change test ordering?" Yes. The old glob.glob(...) returned filesystem-insertion order, which varies across machines and clones; the new code wraps it in sorted(...), so ordering is now alphabetical and stable. The change reduces, not increases, non-determinism.

Test plan

  • From repo root: python3 -m pytest -vvv -k 'xform_ or error_cases' — expect 219 passed, 55 deselected. This is the case that previously produced a silent PASS with zero cases collected; it now works because of the conftest cwd anchor.
  • cd tests && python3 -m pytest -vvv -k 'xform_ or error_cases' — expect 219 passed, 55 deselected. The historical contract still works.
  • From an arbitrary cwd: python3 -m pytest --collect-only -q -k xform_ <repo>/tests/test_transformation.py — expect 109 cases collected, not 0 (validates the silent-PASS-from-wrong-cwd fix).
  • cd tests && ./run-tests.sh — expect 219 passed (validates wrappers still work).
  • python3 -m pytest -vvv 'tests/test_transformation.py::test_xform_cases[topology/input/6pe.yml]' from repo root — expect 1 passed (validates per-case targetability).
  • Temporarily edit one expected fixture (e.g. tests/topology/expected/6pe.yml), re-run the suite, confirm pytest reports "1 failed, 108 passed" instead of aborting at the first mismatch. Revert.
  • In a worktree distinct from the main checkout, run cd tests && python3 -m pytest -k xform_ and confirm it does not silently use the main checkout's netsim (smoke-test by introducing a deliberate divergence between the two checkouts).

The transformation suite had three usability defects that all surface as
"the tests silently lie about what they're testing":

1. From any cwd other than tests/, glob.glob('topology/input/*yml')
   returned [], the for-loop ran zero iterations, and pytest reported
   PASSED. A test file globbing relative paths cannot be safely invoked
   from anywhere except tests/, but AGENTS.md showed exactly that.

2. From a git worktree, 'cd tests && pytest' silently imported the
   netsim package from the venv egg-link (the main checkout) instead of
   the worktree's own code, because sys.path[0]='' resolves to cwd at
   interpreter startup and tests/ has no netsim/. The wrapper scripts
   papered over this with PYTHONPATH=../ but bare pytest did not.

3. All 109 transform cases (and 110 error cases, and the two coverage
   suites) ran inside a single for-loop with one assert. The first
   failing case aborted the rest -- CI and developers saw "1 failed"
   without knowing whether the regression touched 1 case or 50, and -k
   couldn't target an individual case because they weren't pytest nodes.

Changes:
- New tests/conftest.py: prepends the worktree root to sys.path so
  imports always resolve against this checkout's netsim/, and chdir's
  to tests/ in pytest_configure so the existing relative globs work
  from any invocation cwd.
- tests/test_transformation.py: @pytest.mark.parametrize over the four
  glob.glob() lists so each YAML case becomes its own pytest node
  (test_xform_cases[topology/input/foo.yml], etc.). -k can now target
  one case; one failure no longer hides others. test_coverage_verbose_cases
  iterates the glob directly since it's no longer callable as a function
  after parametrize. The unreachable __main__ block (broken signature,
  no callers) was removed.

The run-tests.sh / run-xform.sh / run-xerr.sh / run-coverage-tests.sh
wrappers continue to work unchanged; their -k selectors keep matching
because the function names are preserved.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown
Owner

@ipspace ipspace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job. Thank you!

@ipspace ipspace merged commit 6ec76e2 into ipspace:dev May 15, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants